Requested Changes: Add subpath parameter

Context

During testing, we identified a bug where plugin paths (e.g., github:owner/repo/plugins/sub-plugin) were being rejected by parse_plugin_source() because it validates that GitHub shorthand has only one / in the repo path. Rather than embedding the path in the source string, we've decided to keep path as a separate parameter throughout the entire stack for consistency and clarity.

Changes Needed

openhands-sdk/openhands/sdk/plugin/fetch.py - Add subpath parameter to fetch_plugin():

def fetch_plugin(
    source: str,
    cache_dir: Path | None = None,
    ref: str | None = None,
    update: bool = True,
    subpath: str | None = None,  # NEW PARAMETER
    git_helper: GitHelper | None = None,
) -> Path:
    """Fetch a plugin from a remote source and return the local cached path.

    Args:
        source: Plugin source - "github:owner/repo", git URL, or local path
        cache_dir: Directory for caching
        ref: Optional branch, tag, or commit
        update: If True, update cached repo
        subpath: Optional subdirectory path within the repo
        git_helper: GitHelper instance

    Returns:
        Path to the plugin directory (with subpath applied if specified)
    """
    source_type, url = parse_plugin_source(source)
    
    # ... existing clone/update logic ...
    
    # NEW: Apply subpath if specified (add before final return)
    if subpath:
        final_path = plugin_path / subpath.strip('/')
        if not final_path.exists():
            raise PluginFetchError(
                f"Subdirectory '{subpath}' not found in plugin repository"
            )
        return final_path
    
    return plugin_path

openhands-sdk/openhands/sdk/plugin/plugin.py - Add subpath parameter to Plugin.fetch():

@classmethod
def fetch(
    cls,
    source: str,
    cache_dir: Path | None = None,
    ref: str | None = None,
    update: bool = True,
    subpath: str | None = None,  # NEW PARAMETER
) -> Path:
    """Fetch a plugin from a remote source."""
    return fetch_plugin(
        source, 
        cache_dir=cache_dir, 
        ref=ref, 
        update=update,
        subpath=subpath,  # NEW
    )

This maintains API shape consistency across all layers (see OpenHands/OpenHands#12087 (comment) for the full API design).

@OpenHands Install yourself gh if you don't already have it, and then get all this PR diff and the inline comments to it.

GOAL: answer a code design question: does this feature implementation belong in the sdk package of the SDK (like this PR), in the workspace package of the SDK (Workspace classes), or in the client apps?

I want you to make a throughout research in the SDK packages and try to understand the git-related operations and responsibilities. Answer directly in a github comment.

I'm on it! enyst can track my progress at all-hands.dev

Thanks for flagging this — I dug through the SDK packages to see where “git-ish” responsibilities currently live.

What exists today

SDK (openhands-sdk) already does small, self-contained git + caching for SDK-level assets.

• openhands.sdk.context.skills.utils.update_skills_repository() maintains a local clone under ~/.openhands/cache/skills/... and does git clone/fetch/reset via subprocess.
  • This is not workspace-scoped: it’s a pure “fetch a read-only dependency into a user cache” helper.

Workspace (openhands-workspace) is about running commands / managing environments, not about bringing in additional repos into ~/.openhands caches.

• The workspace layer exposes git introspection APIs (git_changes, git_diff) for the repo inside the workspace, but it does not implement git clone / remote fetching logic. It’s basically “operate on whatever repo is already in the workspace”.

So where should Plugin.fetch() live?

Assessment: Plugin.fetch() belongs in openhands-sdk (the sdk package), but ideally as a shared “remote repo fetch + cache” utility used by both skills + plugins, not in Workspace or client apps.

Why SDK (not Workspace):

• The feature is “resolve a plugin source string → ensure a local cached copy exists → return a filesystem path”. That mirrors the existing public-skills behavior and is independent of any particular workspace implementation (Docker, API remote, cloud, etc).
• The cache location is under the user’s home (~/.openhands/cache/...), which is outside the workspace’s conceptual boundary. Workspaces should generally avoid mutating host-level caches (especially for remote workspaces).
• Workspace APIs today are about a repo that already exists in the workspace (diff/changes), not “bring external dependencies into existence”.

Why not client apps:

• If every client re-implements “git clone with caching + ref checkout + update semantics”, we’ll end up with duplicated, inconsistent behavior. This is a reusable SDK primitive (plugins, skills registry, future org registries, etc).
• Keeping it in the SDK makes it testable and available to multiple clients.

Design suggestion (to reduce duplication)

Right now the PR introduces openhands.sdk.plugin.git_helper.GitHelper while openhands.sdk.git.utils and skills/utils.py already contain git wrappers.

I’d recommend unifying these as:

1. a single openhands.sdk.git abstraction for both "git introspection" and "git fetch/clone" operations, and
2. small domain wrappers:
   • skills calls something like sdk.git.cached_clone_or_update(...)
   • plugin calls the same and adds “parse plugin source → choose cache key/name”.

That keeps git operations centralized and avoids the “we have 2-3 ways to call git” situation.

Summary of work completed

• Installed the GitHub CLI (gh) in the environment (it was not present).
• Retrieved PR feat(plugin): Add Plugin.fetch() for remote plugin fetching and caching #1647 metadata and the full diff (gh pr diff 1647 …) and reviewed the implementation (Plugin.fetch + parsing + caching + new git helper).
• Pulled all PR-level comments (gh pr view --comments) and all inline review comments via the GitHub API endpoint for pull request review comments (/pulls/1647/comments), and reviewed them (notably the concern about duplicating existing git clone/checkout logic and whether this belongs in workspace).
• Performed a repo-wide audit of existing git responsibilities across packages:
  • Found existing SDK git operations and caching behavior in openhands-sdk, specifically openhands.sdk.context.skills.utils.update_skills_repository() which clones/fetches/resets into ~/.openhands/cache/skills.
  • Confirmed openhands-workspace focuses on workspace lifecycle + executing commands and does not implement remote clone/fetch-to-cache logic; SDK workspace exposes git introspection APIs (git_changes, git_diff) for repos already present.
  • Reviewed openhands.sdk.git.* utilities (safe git command wrapper, diff/changes helpers) to understand current patterns.
• Posted a GitHub PR comment answering the design question directly:
  • Conclusion: Plugin.fetch() belongs in openhands-sdk, not Workspace or client apps.
  • Additional recommendation: reduce duplication by centralizing “clone/fetch/update cached repo” primitives under openhands.sdk.git and have both skills and plugin use that shared utility.

Checklist vs user request

• Install gh if missing
• Get PR diff
• Get inline comments
• Research SDK packages for git-related responsibilities
• Answer the design question directly in a GitHub comment

Code changes / pushes

• No repository code changes were made.
• No commits were created.
• Nothing to push.

View full conversation

Looks like there are a few issues preventing this PR from being merged!

• GitHub Actions are failing:
  • Pre-commit checks

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #1647 at branch `feat/plugin-fetch`

Feel free to include any additional details that might help me get this PR into a better state.

_{^{You can manage your notification settings}}

@enyst:

Git Operations Consolidation - Complete

Per your feedback about duplicate git operations, we've completed a full consolidation. Here's what was done:

Summary

We've refactored GitHelper in cached_repo.py to use the existing run_git_command() utility from git/utils.py instead of calling subprocess directly. This eliminates the code duplication you identified.

Changes Made

1. Extended run_git_command() with an optional timeout parameter (default: 30s) to support the varying timeout needs of different git operations (clone: 120s, fetch: 60s, etc.)

2. Refactored GitHelper class - All methods now delegate to run_git_command():

   • clone(), fetch(), checkout(), reset_hard(), get_current_branch()
   • Removed ~70 lines of duplicate subprocess handling code

3. Unified error handling - All git operations now use GitCommandError (the structured exception with command, exit_code, and stderr fields) instead of the simpler GitError with just a message string

4. Updated exports - Added GitCommandError to the plugin module's public API for backwards compatibility

Result: Single Code Path

All git subprocess calls in the SDK now flow through run_git_command():

git/utils.py:run_git_command()     ← Single entry point for all git subprocess calls
    ↑
git/cached_repo.py:GitHelper       ← Used by skills & plugins
    ↑
├── context/skills/utils.py        ← Skills repository caching  
└── plugin/fetch.py                ← Plugin repository caching